feat(workspaces): add workspace logo upload#4136
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Introduces a new uploads context Extends the workspace API and client types/mutations to persist Reviewed by Cursor Bugbot for commit 473785c. Configure here. |
Greptile SummaryThis PR adds workspace logo upload support via a right-click context menu, introduces a dedicated The previous thread concerns — storage cleanup parity with profile-pictures and URL validation — have been acknowledged/addressed. All remaining findings are P2. Confidence Score: 5/5Safe to merge — all findings are P2 style/edge-case concerns with no blocking bugs or security issues Server-side authorization is correct (admin-only upload + PATCH, URL validation, public serve for logos). The DB migration is additive and backwards-compatible. Prior review concerns (storage cleanup parity, arbitrary URL injection) have been acknowledged or addressed. The two remaining findings are code duplication and an edge-case workspaceIdRef drift that only affects audit logs in an unlikely navigation sequence, neither of which blocks shipping. apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-workspace-logo-upload.ts — code duplication and workspaceIdRef drift Important Files Changed
Sequence DiagramsequenceDiagram
actor User
participant Sidebar
participant Hook as useWorkspaceLogoUpload
participant UploadAPI as POST /api/files/upload
participant Storage as workspace-logos bucket
participant PatchAPI as PATCH /api/workspaces/[id]
participant DB as workspace.logo_url
User->>Sidebar: Right-click workspace → Upload logo
Sidebar->>Hook: setTargetWorkspaceId(workspaceId)
Sidebar->>Sidebar: logoTargetWorkspaceIdRef = workspaceId
Sidebar->>User: Open file picker
User->>Sidebar: Select image file
Sidebar->>Hook: handleFileChange(file)
Hook->>Hook: validateFile (size ≤5MB, allowed type)
Hook->>Hook: setPreviewUrl(objectURL)
Hook->>UploadAPI: POST multipart (file, context=workspace-logos, workspaceId)
UploadAPI->>UploadAPI: Verify user is workspace admin
UploadAPI->>Storage: uploadFile(workspace-logos/ts-filename)
Storage-->>UploadAPI: fileInfo.path
UploadAPI->>UploadAPI: recordAudit + captureServerEvent
UploadAPI-->>Hook: fileInfo.path
Hook->>Hook: setPreviewUrl(serverUrl)
Hook->>Sidebar: onUpload(serverUrl)
Sidebar->>PatchAPI: PATCH { logoUrl: serverUrl }
PatchAPI->>PatchAPI: Verify user is workspace admin
PatchAPI->>PatchAPI: Validate logoUrl (starts with / or https://)
PatchAPI->>DB: UPDATE workspace SET logo_url = serverUrl
PatchAPI->>PatchAPI: recordAudit
PatchAPI-->>Sidebar: updated workspace
Sidebar->>Sidebar: invalidate workspaceKeys.lists()
Sidebar->>Sidebar: Re-render with new logoUrl in header
Reviews (6): Last reviewed commit: "refactor: replace raw fetch with useWork..." | Re-trigger Greptile |
7ca4af8 to
1d5656a
Compare
|
Both security findings are now addressed:
|
|
@greptile |
|
@cursor review |
…t for workspace logo uploads
- Add image/webp to ACCEPTED_IMAGE_TYPES in useProfilePictureUpload - Add image/webp to file input accept attributes in whitelabeling settings - Refactor useProfilePictureUpload to use refs for onUpload, onError, and currentImage callbacks, matching the established codebase pattern Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These files were accidentally regressed during rebase conflict resolution, reverting changes from #4027. Restoring to staging versions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0e1b3a0 to
8794e12
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@greptile |
|
@cursor review |
Split the ref sync useEffect so workspaceIdRef only updates when the workspaceId prop changes, not when onUpload/onError callbacks get new references. Prevents setTargetWorkspaceId from being overwritten by a re-render before the file upload completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@cursor review |
|
@greptile |
|
@greptile |
|
@cursor review |
The shared Workspace type requires ownerId and other fields that aren't available from the workspaces API response mapping. Use a Pick type to accurately represent the subset of fields actually constructed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove useState + useEffect + fetch anti-pattern for loading workspaces. Use useWorkspacesQuery from React Query with inline filter for write/admin permissions. Eliminates ~30 lines of manual state management, any casts, and the Pick type workaround. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@cursor review |
|
@greptile |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f05046a. Configure here.
Summary
Type of Change
Testing
Tested manually
Checklist